fix: Adopt sessionToken from save/update response on ParseUser#1139
fix: Adopt sessionToken from save/update response on ParseUser#1139chadpav wants to merge 3 commits into
Conversation
Parse Server mints a fresh session row when password is set on an existing _User and revokeSessionOnPasswordReset is true (default since 9.x). The new sessionToken is embedded in the save response. Without adopting it, the global session in ParseCoreData keeps pointing at the prior session — which the server just destroyed — and every subsequent request fails with invalidSessionToken until the next login. Mirrors PFUser._mergeFromServerWithResult on iOS, which reads PFUserSessionTokenRESTKey out of the response and installs it. The helper is a no-op when the response carries no sessionToken (e.g. a plain field update with no password change), preserving the existing behavior for non-auth saves.
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesSession Token Adoption
Sequence DiagramsequenceDiagram
participant ParseUser
participant ParseClient
participant ParseCoreData
ParseUser->>ParseClient: super.save()/super.update() (HTTP PUT)
ParseClient-->>ParseUser: JSON response (may include sessionToken)
ParseUser->>ParseCoreData: _adoptResponseSessionTokenIfChanged(tokenBefore) -> setSessionId(newToken)
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1139 +/- ##
==========================================
+ Coverage 44.42% 45.14% +0.72%
==========================================
Files 62 62
Lines 3730 3739 +9
==========================================
+ Hits 1657 1688 +31
+ Misses 2073 2051 -22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/dart/test/src/objects/parse_user/parse_user_session_token_test.dart (1)
23-25: ⚡ Quick winRestore
ParseCoreData().sessionIdbetween tests.Each case mutates the singleton session state and leaves it behind, so later tests can inherit leaked auth state. Snapshot the prior value in
setUp()and restore it intearDown().♻️ Suggested change
+ late String? previousSessionId; + setUp(() { + previousSessionId = ParseCoreData().sessionId; client = MockParseClient(); }); + + tearDown(() { + ParseCoreData().sessionId = previousSessionId; + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dart/test/src/objects/parse_user/parse_user_session_token_test.dart` around lines 23 - 25, The tests mutate the singleton ParseCoreData().sessionId and leak state across cases; modify the test harness so setUp() captures the current ParseCoreData().sessionId into a local variable (e.g., previousSessionId) and tearDown() restores it to ParseCoreData().sessionId; keep the existing MockParseClient() assignment in setUp() and ensure teardown always runs to reset session state between tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/dart/test/src/objects/parse_user/parse_user_session_token_test.dart`:
- Around line 23-25: The tests mutate the singleton ParseCoreData().sessionId
and leak state across cases; modify the test harness so setUp() captures the
current ParseCoreData().sessionId into a local variable (e.g.,
previousSessionId) and tearDown() restores it to ParseCoreData().sessionId; keep
the existing MockParseClient() assignment in setUp() and ensure teardown always
runs to reset session state between tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b96c137-5691-4256-aa71-a15e104038db
📒 Files selected for processing (2)
packages/dart/lib/src/objects/parse_user.dartpackages/dart/test/src/objects/parse_user/parse_user_session_token_test.dart
Tests in this group mutate the singleton ParseCoreData().sessionId and were not restoring it, leaking state across cases. Capture the prior session in setUp() and restore it in tearDown() so each test starts from a known baseline. Addresses CodeRabbit nit on PR parse-community#1139.
CI runs `dart format --set-exit-if-changed` on Dart 3.10 stable; the test file was written under a newer formatter style and tripped that check. Apply the Dart 3.10 conventions (inline test name, no trailing comma in test() argument list).
|
The label |
|
@mtrezza anything I can do to help get these across? This sessionToken not getting saved is a high severity issue in the Flutter SDK. |
Issue
No existing issue.
Approach
Split out from #1136 at maintainer request. This PR isolates the
session-token adoption concern.
Parse Server mints a fresh
_Sessionrow whenpasswordis set onan existing
_User(withrevokeSessionOnPasswordReset = true, thecurrent default in parse-server 9.x). The new
sessionTokenisembedded in the save response. Without adopting it, the global
session in
ParseCoreDatakeeps pointing at the prior session —which the server just destroyed — and every subsequent request
fails with
invalidSessionTokenuntil the next login.This mirrors
PFUser._mergeFromServerWithResulton iOS, which readsPFUserSessionTokenRESTKeyout of the response and installs it.Implemented as
_adoptResponseSessionTokenIfChanged(String?)invoked from
save()andupdate(). The helper is a no-op whenthe response carries no
sessionToken(e.g. a plain field updatewith no password change), preserving the existing behavior for
non-auth saves.
Tasks
Behavior change disclosure
When a
_Usersave/update response carries asessionTokenfield(i.e. parse-server minted a new session), the SDK installs it as the
active session via
ParseCoreData().setSessionId(...). Previouslythe new token was merged into the local user object but the global
session continued pointing at the prior token.
Tests
packages/dart/test/src/objects/parse_user/parse_user_session_token_test.dart— 3 tests covering:
save()installs a newsessionTokenfrom the response into the global sessionsave()is a no-op when the response carries nosessionTokenupdate()installs a newsessionTokenfrom the responseAll 209 tests in
packages/dart/test/pass.Summary by CodeRabbit
Bug Fixes
Tests